🐛 Fix ClusterObjectSet ref resolution for Secrets outside system namespace by pedjak · Pull Request #2624 · operator-framework/operator-controller

pedjak · 2026-04-01T16:38:44Z

Description

The controller cache only watches the system namespace, causing ClusterObjectSet ref resolution to fail when Secrets are stored in other namespaces.

This PR:

Introduces a client wrapper that falls back to direct API calls for Secret reads outside the system namespace
Grants cluster-wide Secret get permission when BoxcutterRuntime is enabled
Adds an e2e scenario covering direct ClusterObjectSet creation with ref-based objects stored in Secrets

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2026-04-01T16:38:50Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`f8a4c02`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/69ce56a936d6220008e07d2f
😎 Deploy Preview	https://deploy-preview-2624--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

Adds an e2e scenario to validate direct ClusterObjectSet creation where phase objects are externalized in a Secret via ref, and improves e2e cleanup to handle additional (non-CE/COS) resources and namespaced deletions.

Changes:

Add revision.feature scenario for ClusterObjectSet referencing Secret-stored objects via ref.
Track additional applied resources for cleanup in ResourceIsApplied.
Extend cleanup deletion to include -n <namespace> when applicable.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
test/e2e/steps/steps.go	Tracks non-CE/COS applied resources (incl. namespace) for cleanup.
test/e2e/steps/hooks.go	Adds namespace field to tracked resources and uses it when deleting.
test/e2e/features/revision.feature	Adds new e2e scenario for COS creation via Secret `ref` objects.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T16:45:25Z

test/e2e/steps/hooks.go

+				args = append(args, "-n", res.namespace)
+			}
+			if _, err := k8sClient(args...); err != nil {
 				logger.Info("Error deleting resource", "name", res.name, "namespace", sc.namespace, "stderr", stderrOutput(err))


The log field namespace uses sc.namespace, but the deletion is performed with res.namespace (and some resources may be cluster-scoped / other namespaces). This makes failure logs misleading. Log res.namespace (and optionally also log sc.namespace under a different key if you need scenario context).

Suggested change

logger.Info("Error deleting resource", "name", res.name, "namespace", sc.namespace, "stderr", stderrOutput(err))

logger.Info("Error deleting resource", "name", res.name, "namespace", res.namespace, "scenarioNamespace", sc.namespace, "stderr", stderrOutput(err))

Copilot · 2026-04-01T16:45:26Z

test/e2e/steps/steps.go

+		sc.addedResources = append(sc.addedResources, resource{
+			name:      res.GetName(),
+			kind:      strings.ToLower(res.GetKind()),
+			namespace: res.GetNamespace(),


For namespaced resources applied without an explicit metadata.namespace in the YAML, res.GetNamespace() will be empty (because this is parsed from the manifest). With the new cleanup logic, those resources would be deleted without -n, likely leaving them behind. Consider defaulting namespace to the scenario/test namespace when res.GetNamespace() is empty (at least for this e2e harness), so cleanup reliably targets the namespace resources were applied into.

Suggested change

sc.addedResources = append(sc.addedResources, resource{

name: res.GetName(),

kind: strings.ToLower(res.GetKind()),

namespace: res.GetNamespace(),

namespace := res.GetNamespace()

if namespace == "" {

namespace = sc.namespace

}

sc.addedResources = append(sc.addedResources, resource{

name: res.GetName(),

kind: strings.ToLower(res.GetKind()),

namespace: namespace,

Copilot · 2026-04-01T16:45:26Z

test/e2e/steps/hooks.go

 		go func(res resource) {
-			if _, err := k8sClient("delete", res.kind, res.name, "--ignore-not-found=true"); err != nil {
+			args := []string{"delete", res.kind, res.name, "--ignore-not-found=true"}
+			if res.namespace != "" {
+				args = append(args, "-n", res.namespace)
+			}
+			if _, err := k8sClient(args...); err != nil {
 				logger.Info("Error deleting resource", "name", res.name, "namespace", sc.namespace, "stderr", stderrOutput(err))
 			}
 		}(r)


Cleanup deletes all resources concurrently. With the PR tracking more resources, it becomes more likely that namespace deletion races with deletions of namespaced resources, creating intermittent cleanup failures/noise. Consider deleting in a deterministic order (e.g., delete tracked resources first, then delete the namespace last) and/or avoiding goroutines here so the namespace isn’t removed while other deletes are still in-flight.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The controller cache only watches the system namespace, causing ref resolution to fail when Secrets are stored in other namespaces. Fix by introducing a client wrapper that falls back to direct API calls for Secret reads outside the system namespace, and grant cluster-wide Secret get permission when BoxcutterRuntime is enabled. Adds an e2e scenario covering this path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codecov · 2026-04-02T12:05:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.86%. Comparing base (735b41e) to head (f8a4c02).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2624      +/-   ##
==========================================
+ Coverage   63.19%   68.86%   +5.67%     
==========================================
  Files         139      139              
  Lines        9902     9910       +8     
==========================================
+ Hits         6258     6825     +567     
+ Misses       3147     2573     -574     
- Partials      497      512      +15

Flag	Coverage Δ
e2e	`37.50% <0.00%> (-0.11%)`	⬇️
experimental-e2e	`52.17% <100.00%> (+41.04%)`	⬆️
unit	`53.50% <0.00%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tmshort · 2026-04-02T14:56:51Z

/approve

rashmigottipati

/lgtm

openshift-ci · 2026-04-02T14:58:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rashmigottipati, tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI review requested due to automatic review settings April 1, 2026 16:38

openshift-ci bot requested review from fgiudici and grokspawn April 1, 2026 16:38

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot started reviewing on behalf of pedjak April 1, 2026 17:12 View session

pedjak force-pushed the e2e-cos-ref-objects branch from 4b980cb to 20c5bc8 Compare April 2, 2026 11:34

pedjak changed the title ~~🌱 Add e2e test for direct ClusterObjectSet creation with Secret-stored objects~~ Fix ClusterObjectSet ref resolution for Secrets outside system namespace Apr 2, 2026

pedjak changed the title ~~Fix ClusterObjectSet ref resolution for Secrets outside system namespace~~ 🐛 Fix ClusterObjectSet ref resolution for Secrets outside system namespace Apr 2, 2026

Copilot AI review requested due to automatic review settings April 2, 2026 11:36

pedjak force-pushed the e2e-cos-ref-objects branch from 20c5bc8 to 5af3951 Compare April 2, 2026 11:36

Copilot started reviewing on behalf of pedjak April 2, 2026 11:37 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

pedjak force-pushed the e2e-cos-ref-objects branch from 5af3951 to f8a4c02 Compare April 2, 2026 11:44

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2026

rashmigottipati approved these changes Apr 2, 2026

View reviewed changes

openshift-ci bot assigned rashmigottipati Apr 2, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 2, 2026

openshift-merge-bot bot merged commit a6989b7 into operator-framework:main Apr 2, 2026
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Fix ClusterObjectSet ref resolution for Secrets outside system namespace#2624

🐛 Fix ClusterObjectSet ref resolution for Secrets outside system namespace#2624
openshift-merge-bot[bot] merged 1 commit intooperator-framework:mainfrom
pedjak:e2e-cos-ref-objects

pedjak commented Apr 1, 2026 •

edited

Loading

Uh oh!

netlify bot commented Apr 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

codecov bot commented Apr 2, 2026 •

edited

Loading

Uh oh!

tmshort commented Apr 2, 2026

Uh oh!

rashmigottipati left a comment

Uh oh!

openshift-ci bot commented Apr 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	logger.Info("Error deleting resource", "name", res.name, "namespace", sc.namespace, "stderr", stderrOutput(err))
	logger.Info("Error deleting resource", "name", res.name, "namespace", res.namespace, "scenarioNamespace", sc.namespace, "stderr", stderrOutput(err))

Conversation

pedjak commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Checklist

Uh oh!

netlify bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

codecov bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tmshort commented Apr 2, 2026

Uh oh!

rashmigottipati left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Apr 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pedjak commented Apr 1, 2026 •

edited

Loading

netlify bot commented Apr 1, 2026 •

edited

Loading

codecov bot commented Apr 2, 2026 •

edited

Loading